feat: evacuation screen and small refactor of file structure in digit… #489

tomasz-trela · 2024-12-14T11:57:25Z

I refactored file structure a little bit. I decided to do this in tab (not in another screen) because when it's look more consistent. I know it's not the best practice in ux, but I think not many people will use this tab (so ui will benefit from this).

I'm open for any suggestions.

Copilot reviewed 2 out of 21 changed files in this pull request and generated no comments.

Files not reviewed (19)

lib/config/ui_config.dart: Language not supported
lib/features/digital_guide_view/data/models/digital_guide_response.dart: Language not supported
lib/features/digital_guide_view/data/models/digital_guide_response_extended.dart: Language not supported
lib/features/digital_guide_view/data/repository/digital_guide_repository.dart: Language not supported
lib/features/digital_guide_view/presentation/digital_guide_view.dart: Language not supported
lib/features/digital_guide_view/presentation/widgets/accessibility_button.dart: Language not supported
lib/features/digital_guide_view/presentation/widgets/digital_guide_data_source_link.dart: Language not supported
lib/features/digital_guide_view/presentation/widgets/digital_guide_features_section.dart: Language not supported
lib/features/digital_guide_view/presentation/widgets/digital_guide_go_to_button.dart: Language not supported
lib/features/digital_guide_view/presentation/widgets/headlines_section.dart: Language not supported
lib/features/digital_guide_view/presentation/widgets/report_change_button.dart: Language not supported
lib/features/digital_guide_view/tabs/amenities/presentation/amenities_expansion_tile_content.dart: Language not supported
lib/features/digital_guide_view/tabs/entraces/data/models/digital_guide_entrace.dart: Language not supported
lib/features/digital_guide_view/tabs/entraces/data/repository/entraces_repository.dart: Language not supported
lib/features/digital_guide_view/tabs/evacuation/evacuation_widget.dart: Language not supported
lib/features/digital_guide_view/tabs/models/digital_guide_entrace.dart: Language not supported
lib/features/digital_guide_view/tabs/surrounding/data/repository/surrounding_repository.dart: Language not supported
lib/features/digital_guide_view/tabs/surrounding/presentation/surroundings_expansion_tile_content.dart: Language not supported
lib/features/navigator/app_router.dart: Language not supported

simon-the-shark

Good job Tomuś!

I like the file structure. Only thing is that you have some duplicate trash files in my opinion, DigitalGuideEntrace models are twice in the filesystem and the LocalizationExpansionTileContent inside localization in not in the tabs. Rest look good.

Few notes left, one is a big missing one from the previous @24bartixx big PR - idk how I missed that.

I also propose to create a separate evacutation repo, but not sure if I got everything right (didn't study the responses in very much detail)

Tell me what you think!

lib/features/digital_guide_view/data/repository/digital_guide_repository.dart

lib/features/digital_guide_view/presentation/widgets/digital_guide_go_to_button.dart

lib/features/digital_guide_view/tabs/models/digital_guide_entrace.dart

lib/features/digital_guide_view/presentation/widgets/digital_guide_go_to_button.dart

lib/features/digital_guide_view/tabs/entraces/data/models/digital_guide_entrace.dart

tomasz-trela · 2024-12-14T21:02:25Z

I can't create separate repo for evacuation because evacuation is a property of building( buildings endpoint).

simon-the-shark · 2024-12-14T21:09:26Z

I can't create separate repo for evacuation because evacuation is a property of building( buildings endpoint).

Your evacuation repo can use the building data provider:

@riverpod
Future<EvacuationModel> evacuationRepo(Ref ref, int id) async {
  final building = await ref.watch(getDigitalGuideDataProvider(id).future);
  final evacuationMapUrl =
      await ref.watch(getImageUrlProvider(building.evacuationMapId).future);

  // ...
  //...
  //...
  
  return EvacuationModel(
    //...
  );
}

or am i wrong?

tomasz-trela · 2024-12-14T21:23:30Z

I can't create separate repo for evacuation because evacuation is a property of building( buildings endpoint).

Your evacuation repo can use the building data provider:

@riverpod
Future<EvacuationModel> evacuationRepo(Ref ref, int id) async {
  final building = await ref.watch(getDigitalGuideDataProvider(id).future);
  final evacuationMapUrl =
      await ref.watch(getImageUrlProvider(building.evacuationMapId).future);

  // ...
  //...
  //...
  
  return EvacuationModel(
    //...
  );
}

or am i wrong?

Now I understand

tomasz-trela · 2024-12-14T22:34:15Z

Done

lib/config/ui_config.dart

simon-the-shark

so at the moment you're loading everything at first load and I would consider if loading things should happen when we open the proper extension tile. imo we should do things as lazy as possible. but we can merge it now and correct in next pr too

simon-the-shark · 2024-12-16T15:00:30Z

and before we make a review, please resolve conflicts to reduce diff size

…al guide

simon-the-shark

basically LGTM, still I would maybe at least test the behaviour in case where we would only download this things when we open the specific tab and not all at first load, cause with time this is gonna be a lot of requests. But this require some repository rearranging - and maybe I'll handle this when I have a moment (someone else also can :P)

Merging this for now.

simon-the-shark · 2024-12-18T17:57:07Z

lib/widgets/my_error_widget.dart

@@ -14,6 +14,8 @@ import "../features/parkings_view/widgets/offline_parkings_view.dart";
 import "../gen/assets.gen.dart";
 import "../theme/app_theme.dart";

+// MEGA TEST NICE // TODO(simon-the-shark): delete this comment


you was supposed to delete this comment 😭

simon-the-shark · 2024-12-18T17:58:11Z

lib/features/digital_guide_view/data/repository/digital_guide_repository.dart

 import "../models/digital_guide_response.dart";
 import "../models/digital_guide_response_extended.dart";

 part "digital_guide_repository.g.dart";

 @riverpod
-Future<DigitalGuideResponseExtended> getDigitalGuideData(
+Future<DigitalGuideResponseExtended> getDigitalGuideDataExtended(


I don't know who changed the convention, but I liked "digitalGuideRepository' convention more than 'getX' - but maybe that's just me

tomasz-trela requested review from simon-the-shark and mikolaj-jalocha December 14, 2024 11:57

tomasz-trela self-assigned this Dec 14, 2024

tomasz-trela linked an issue Dec 14, 2024 that may be closed by this pull request

feat (digital-guide): add evacuation tile #472

Closed

simon-the-shark requested a review from Copilot December 14, 2024 12:58

Copilot AI reviewed Dec 14, 2024

View reviewed changes

simon-the-shark added the enhancement New feature or request label Dec 14, 2024

simon-the-shark added this to the v1.2.0 - Mr. Shark, We Have to Cook milestone Dec 14, 2024

simon-the-shark reviewed Dec 14, 2024

View reviewed changes

tomasz-trela requested a review from simon-the-shark December 14, 2024 22:34

mikolaj-jalocha reviewed Dec 15, 2024

View reviewed changes

lib/config/ui_config.dart Show resolved Hide resolved

simon-the-shark reviewed Dec 16, 2024

View reviewed changes

Tomasz Trela added 6 commits December 17, 2024 21:36

feat: evacuation screen and small refactor of file structure in digit…

1834c67

…al guide

fix: padding

6bdaa05

chore: fix linter complaints

d005d11

chore: fix linter complaints

fcbad0c

refactor/use providers

ceda67e

chore: fix linter complaints

893fc13

simon-the-shark force-pushed the feat/evacuation-screen branch 2 times, most recently from 30c0889 to 66b6c16 Compare December 17, 2024 20:42

Tomasz Trela added 5 commits December 17, 2024 21:44

feat/generalize parameters

7b82d57

chore: fix linter complaints

11c77a3

chore: fix linter complaints

7ee00e5

refactor: add evacuation repo

227c894

chore: fix linter complaints

0dd6eb9

simon-the-shark force-pushed the feat/evacuation-screen branch from 66b6c16 to 0dd6eb9 Compare December 17, 2024 20:45

fix: rebase mistakes

dcb3289

tomasz-trela requested a review from simon-the-shark December 18, 2024 13:34

simon-the-shark added 2 commits December 18, 2024 19:04

refactor(digital-guide): move localizations to tabs subfolder

1a7bf95

refactor(digital-guide): remove useless comment

9373e9d

simon-the-shark previously approved these changes Dec 18, 2024

View reviewed changes

simon-the-shark dismissed their stale review via 9373e9d December 18, 2024 18:08

simon-the-shark merged commit 216413a into main Dec 18, 2024
2 checks passed

simon-the-shark deleted the feat/evacuation-screen branch December 18, 2024 18:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: evacuation screen and small refactor of file structure in digit… #489

feat: evacuation screen and small refactor of file structure in digit… #489

tomasz-trela commented Dec 14, 2024

simon-the-shark left a comment

tomasz-trela commented Dec 14, 2024

simon-the-shark commented Dec 14, 2024

tomasz-trela commented Dec 14, 2024

tomasz-trela commented Dec 14, 2024

simon-the-shark left a comment

simon-the-shark commented Dec 16, 2024

simon-the-shark left a comment

simon-the-shark Dec 18, 2024

simon-the-shark Dec 18, 2024

feat: evacuation screen and small refactor of file structure in digit… #489

feat: evacuation screen and small refactor of file structure in digit… #489

Conversation

tomasz-trela commented Dec 14, 2024

Choose a reason for hiding this comment

simon-the-shark left a comment

Choose a reason for hiding this comment

tomasz-trela commented Dec 14, 2024

simon-the-shark commented Dec 14, 2024

tomasz-trela commented Dec 14, 2024

tomasz-trela commented Dec 14, 2024

simon-the-shark left a comment

Choose a reason for hiding this comment

simon-the-shark commented Dec 16, 2024

simon-the-shark left a comment

Choose a reason for hiding this comment

simon-the-shark Dec 18, 2024

Choose a reason for hiding this comment

simon-the-shark Dec 18, 2024

Choose a reason for hiding this comment